While I do like the idea of enforcing an order for the lifecycle hooks, I'm not sure whether this is the right way to do it. This is possibly just adding confusion, since we already have a lifecycle-hooks group in there.
I think that perhaps a better approach would be to move model into the lifecycle-hooks group and then add additional logic for sorting lifecycle-hooks themselves.
We could add a configuration option for those hooks, by making it accept an array, like

'lifecycle-hooks': ['beforeModel', 'model', 'afterModel', 'setupController']

I mostly agree with @kevinkucharczyk , but I was also thinking about getting rid of lifecycle-hooks group, then we could come up with the following configuration:

ember/order-in-routes: [2, {
  order: [
    'service',
    'inherited-property',
    'property',
    'beforeModel',
    'model',
    'afterModel',
    ['setupController', 'didTransition', '...']
    'actions',
    'method',
  ]
}]

This way we would take advantage of currently available grouping feature and all other explicitly listed hooks would be treated equally.

@kevinkucharczyk @michalsnik yeah, the idea was we should remove all lifecycle hooks and list them explicitly for ordering. This PR is not meant to be merged yet, but I wanted to get feedback on how I was doing things before I continued down the path. If you are both happy with how I implemented beforeModel and afterModel, I will complete the rest of the hooks.

@kevinkucharczyk @michalsnik I added more route hooks, but not sure if I have gotten them all. It feels very verbose having a separate method to check like isBeforeModelProp. Shouldn't we just have one method that checks all route props and returns the mapping or something? I was just trying to follow how model was done. Please let me know your thoughts, so we can tweak this if necessary and get it in 😄

@rwwagner90 I agree, it is quite verbose. Perhaps you could define one isRouteLifecycleHook method, instead of a bunch of separate methods for each hook? I don't think we'll need those hook-specific methods in other places, so it should be fine.
We could later introduce similar methods for component hooks, like isComponentLifeCycleHook.

We already have isComponentLifecycleHook helper: https://github.com/ember-cli/eslint-plugin-ember/blob/master/lib/utils/ember.js#L204 as well as isRouteDefaultMethod with all methods available on routes (https://github.com/ember-cli/eslint-plugin-ember/blob/master/lib/utils/ember.js#L271) It might be worth using them instead :)

Then simple:

else if (ember.isRouteDefaultMethod(node)) {
  return node.key.name;
} 

might be enough

Thanks @michalsnik! Though I think it would still be good to introduce a separate method for just the route lifecycle hooks. isRouteDefaultMethod checks a lot of stuff, which we don't want/need here.

@kevinkucharczyk @michalsnik thanks for those suggestions! I made an isRouteLifecycleHook method, and made it much less verbose. How is this looking now? Anything else I should do? The one verbose thing left, that we could maybe clean up, is this:

const NAMES = {
  service: 'service injection',
  property: 'property',
  'single-line-function': 'single-line function',
  'multi-line-function': 'multi-line function',
  observer: 'observer',
  beforeModel: 'lifecycle hook',
  model: '"model" hook',
  afterModel: 'lifecycle hook',
  serialize: 'lifecycle hook',
  redirect: 'lifecycle hook',
  activate: 'lifecycle hook',
  setupController: 'lifecycle hook',
  renderTemplate: 'lifecycle hook',
  resetController: 'lifecycle hook',
  deactivate: 'lifecycle hook',
  'lifecycle-hook': 'lifecycle hook',
  actions: 'actions hash',
  method: 'method',
  unknown: 'unknown property type',
  attribute: 'attribute',
  relationship: 'relationship',
  'query-params': 'property'
};

@kevinkucharczyk @michalsnik any other tweaks you would like? I can start on the component one as well, if this looks good 😄

Additionally please fix eslint issue so the build is green @rwwagner90 :)

@michalsnik I removed those two methods and fixed ESLint. Please let me know if there is anything else I should do.

Also, I suppose I should update the docs to reflect these new changes, right?

Yeah, please update docs too.

I'm only worried about backwards compatibility. I would preferably release it in minor update, but in it's current state it will most likely be a breaking change. If we make sure however that lifecycle-hook still works and is properly detected as any other lifecycle hook we could release it without breaking any apps' build. We'd need to also change the default order to be:

[
  'property',
  'single-line-function',
  'multi-line-function',
  'model',
  [
    'beforeModel',
    'lifecycle-hook',
    'afterModel',
    'serialize',
    'redirect',
    'activate',
    'setupController',
    'renderTemplate',
    'resetController',
    'deactivate'
  ],
  'actions',
  'method'
]

So summarising:
I think we should support both configs, the new one ☝️ and the previous one 👇 :

[
  'property',
  'single-line-function',
  'multi-line-function',
  'model',
  'lifecycle-hook',
  'actions',
  'method'
]

This way we'll be able to introduce this change as in the minor release. And then together with new rules that we added recently we'll update default recommendations and do one major release. What do you think guys? @rwwagner90 @kevinkucharczyk

@michalsnik that sounds like a lot of work to support both. I'd prefer to not do that. We could just update models and components and docs all as part of this PR too, and get it in for the next breaking change.

We could do that, but the change might not be so complicated after all, all you'd need to do is to parse settings and replace lifecycle-hook with array of all lifecycle hooks except model. That's my first thought, let me know how do you feel about it :)

@michalsnik I think I implemented what you are referring to in my last commit. Please let me know if you think this will work!

We should add more tests now, as we can pass different settings per test and make sure it works as expected with both settings :)

But overall looks good, I expect it to work properly. Though I'll test it manually too

@michalsnik there is a test that I accidentally left lifecycle-hook in that works. I can add some more tests though.

@michalsnik I added a couple more tests, and updated the docs. Please let me know if I should do anything else 😄

@michalsnik anything else I need to do before we can merge this in? Should I do components and models as part of the same PR or do them separate later?

@michalsnik @kevinkucharczyk anything stopping us from merging this?

@michalsnik @kevinkucharczyk bump

@michalsnik @kevinkucharczyk haven't heard from you guys in awhile. Could one of you please give this one more quick look and merge it in? I think it's good to go.

Hey, sorry @rwwagner90 I've been quite busy recently and now I'm on a vacation. I'll take a look in the next week, I hope it's fine :)

@michalsnik that's fine, didn't realize you were on vacation. Thanks for the reply 😄

@michalsnik by next week, did you mean this week or the following week?

Alright, I think we're good @rwwagner90 . I resolved some conflicts and will merge it as soon as the travis check passes 🚀 Great job! And sorry for such a delay.

Thanks @michalsnik! I appreciate your help and the merge 😄 . I just put up another PR to handle lifecycle hook ordering in components.